ref(slack): Compact Slack issue alert message layout #105994
Conversation
github-actions
bot
added
the
Scope: Backend
leeandher
force-pushed
the
leander/eco-1315-improve-slack-issue-alert-message
branch
from
17702fa to
5f79552
Compare
Add tests for the slack-compact-alerts feature flag behavior: - Verify AI summary displays as "Initial Guess:" context block - Verify suggested assignees appear in compact format Also fix nested quote syntax error in get_issue_summary_text(). Refs ECO-1315 Co-Authored-By: Claude <[email protected]>
Store the feature flag result in an instance variable at the start of build() instead of checking it 8 times throughout the method. Also add a basic test for the compact layout without AI summary dependencies. Refs ECO-1315 Co-Authored-By: Claude <[email protected]>
The compact alerts refactor moved the action_text block from before the context block to after it. Update test assertions to use the correct block indices (blocks[2] -> blocks[3] for action_text). Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
This test is very fragile, but would be a much bigger initiative to change for now, so I just resolved the indices. We were rendering the action_text block ('issue resolved by X') before the context on tags/counts -- when in reality it should be replacing the action block which is underneath it.
That correction, caused this rewrite :/
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Add validation to prevent appending an empty context block when both get_context() returns empty and there are no suggested assignees. This mirrors the guard logic in the non-compact code path and prevents potential Slack API validation errors. Co-Authored-By: Claude <[email protected]>
| context_text = get_context(self.group, self.rules) | ||
|
|
||
| if suggested_assignees: | ||
| suggested_text = ", ".join(suggested_assignees) |
There was a problem hiding this comment.
Do we have a maximum number of assignees elsewhere in the system? I'm curious if we should truncate this list to the top N or something.
| suggested_assignees = [] | ||
| if event_for_tags: | ||
| suggested_assignees = get_suggested_assignees( | ||
| self.group.project, event_for_tags, assignee | ||
| ) | ||
|
|
||
| if self._is_compact: | ||
| if group_context_block := self.get_group_context_block( | ||
| suggested_assignees=suggested_assignees | ||
| ): | ||
| blocks.append(group_context_block) | ||
| else: | ||
| # add event count, user count, substate, first seen | ||
| context = get_context(self.group, self.rules) | ||
| if context: | ||
| blocks.append(self.get_context_block(context)) | ||
|
|
||
| # If an action has been taken, add the text for it (e.g. "Issue resolved by <@U0234567890>") | ||
| if self.actions: | ||
| blocks.append(self.get_markdown_block(action_text)) | ||
|
|
||
| # build actions | ||
| actions = [] |
There was a problem hiding this comment.
I get that this is temporary, but I always find it slightly easier to split these into methods for each boolean value/ff that we can maintain separately wherever possible. Common pieces can be put into helper methods to make this code more readable as well. It just makes changing the implementations slightly cleaner + individually testable.
Adds a slack-compact-alerts flag for the new alert layout. The compact layout won't affect most customers, key differences are
Hard to display all the differences without setting them up each feature and preventing race conditions (i.e. alert fires before suggested assignee is determined), so we can verify with tests and in production via the flag.